Skip to content

fix: permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions - #277

Open
antosubash wants to merge 3 commits into
mainfrom
pi/issue-269-permissions-put-api-permissions-roles-id
Open

fix: permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions#277
antosubash wants to merge 3 commits into
mainfrom
pi/issue-269-permissions-put-api-permissions-roles-id

Conversation

@antosubash

Copy link
Copy Markdown
Owner

Closes #269

Headless Pi worker

This is a draft pull request created from an issue carrying the approval label.
It is never merged automatically and requires human review.

Agent summary

Implementation

  • Made RolePermissionsUpdate.permissions required.
  • Missing permissions now returns HTTP 422 without changing assignments.
  • Explicit {"permissions": []} remains a valid clear-all operation.
  • Added API regression coverage for preservation and deliberate clearing.

Changed areas

  • modules/permissions/permissions/contracts/schemas.py
  • modules/permissions/tests/test_permissions_module.py

Verification

  • uv run pytest modules/permissions/tests29 passed
  • Ruff format/check — passed
  • uv run ty check ...passed
  • git diff --checkpassed

Visual evidence: none required.
Risks/blockers: none.

Local visual evidence

When requested, screenshots, traces, videos, and GIFs are stored only under the worker worktree's ignored
.qa/issues/269/ directory. GitHub CLI cannot attach those files; inspect them on the worker host
or attach selected evidence manually.

…ssions' field silently wipes all role permissions (#269)
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 21, 2026

Copy link
Copy Markdown

Deploying simple-module-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0a8a8f0
Status: ✅  Deploy successful!
Preview URL: https://55caaf69.simple-module-python.pages.dev
Branch Preview URL: https://pi-issue-269-permissions-put.simple-module-python.pages.dev

View logs

@antosubash

Copy link
Copy Markdown
Owner Author

🔧 CI repair attempt 1 pushed. I will monitor the new checks automatically.

Root cause

The new regression test pushed test_permissions_module.py to 318 lines, violating the 300-line cap.

Changes

  • Moved the regression test into:
    • modules/permissions/tests/test_permissions_role_update.py
  • Reduced test_permissions_module.py to 287 lines.
  • No production code or CI workflows changed.

Verification

  • File-size check: passed.
  • Permissions tests: 29 passed.
  • make ci-python-lint: passed.
  • make ci-python-typecheck: passed.
  • make ci-js-lint: passed.
  • make ci-js-typecheck: passed.
  • make test-js: 103 passed.
  • Full make test: 2045 passed, but 3 unrelated S3 tests failed with Moto returning HTTP 403 during bucket setup.

Remaining risk: full Python test suite remains blocked by the unrelated Moto/S3 environment failure.

@antosubash

Copy link
Copy Markdown
Owner Author

✅ CI checks passed after 1 automatic repair attempt.

@antosubash
antosubash marked this pull request as ready for review August 28, 2026 19:50
@antosubash

Copy link
Copy Markdown
Owner Author

✅ CI checks passed for this pull request.

@antosubash antosubash added bug Something isn't working pi-pr-open The headless Pi worker opened a draft pull request labels Aug 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working pi-pr-open The headless Pi worker opened a draft pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

permissions: PUT /api/permissions/roles/{id} with missing 'permissions' field silently wipes all role permissions

1 participant